Skip to content

Conversation

@CyberVy
Copy link
Contributor

@CyberVy CyberVy commented Feb 23, 2025

The properties _callback_tensor_inputs of StableDiffusionXLControlNetImg2ImgPipeline,StableDiffusionXLControlNetInpaintPipeline,StableDiffusionXLControlNetUnionImg2ImgPipeline,StableDiffusionXLControlNetUnionInpaintPipeline are missing an important element controlnet_image, which makes it impossible to retrieve controlnet_image from callback_kwargs of callback_on_step_end.

This PR is to fix this issue.

@CyberVy CyberVy changed the title Callback Tensor Inputs of the SDXL Controlnet Inpaint and Img2img Pipelines are missing "controlnet_image". Fix Callback Tensor Inputs of the SDXL Controlnet Inpaint and Img2img Pipelines are missing "controlnet_image". Feb 23, 2025
@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 24, 2025

I've noticed that pipelines related to StableDiffusionControlnetPipeline have the same bug.
This bug also causes the classes in diffusers.callback to not work properly.
I think I will fix the bug about them in another PR.
This PR only fixes SXDL pipeline related bug.

@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 24, 2025

@yiyixuxu @sayakpaul
Could you please help me to review this PR. I'll be very grateful. 🙏

Copy link
Collaborator

@yiyixuxu yiyixuxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR @CyberVy
change looks good to me!

@yiyixuxu
Copy link
Collaborator

can you run make style and make fix-copies?

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 24, 2025

@yiyixuxu Thank you!
I've run make style and make fix-copies.

@CyberVy CyberVy requested a review from yiyixuxu February 24, 2025 17:36
@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 24, 2025

@yiyixuxu Thank you!

I've run make style and make fix-copies.

The checks failed again because of the same reason, although I've done the things above. Is there something wrong with the checks? Or did I do something wrong?

@asomoza
Copy link
Member

asomoza commented Feb 25, 2025

@CyberVy Hi, are you using windows?

@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 25, 2025

@CyberVy Hi, are you using windows?

@asomoza No.

@asomoza
Copy link
Member

asomoza commented Feb 25, 2025

seems the tests are passing now, so no problem, just update the branch and we're got to go after the tests pass, if they don't pass are you ok if I finish it so we can merge?

@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 25, 2025

seems the tests are passing now, so no problem, just update the branch and we're got to go after the tests pass, if they don't pass are you ok if I finish it so we can merge?

Thank you. The checks passed.:)

@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 25, 2025

I think there is no problem, can we merge now? @asomoza

@asomoza
Copy link
Member

asomoza commented Feb 25, 2025

oh I though it was good but now I see that (probably because of the make style) two more files were added which aren't related to this PR. Can you remove them please?

@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 25, 2025

oh I though it was good but now I see that (probably because of the make style) two more files were added which aren't related to this PR. Can you remove them please?

OMG. Yes, it's because of make style.

@asomoza
Copy link
Member

asomoza commented Feb 25, 2025

you deleted them, what I meant was to remove them from the tracking, you can just discard the changes to those two files.

@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 25, 2025

you deleted them, what I meant was to remove them from the tracking, you can just discard the changes to those two files.

🙏Sorry, this is my first PR, so I'm not familar with it. Can I create those deleted files by copying them from the main branch?

@asomoza
Copy link
Member

asomoza commented Feb 25, 2025

Sorry, this is my first PR, so I'm not familar with it.

No problem, we all had a first PR so no worries.

Can I create those deleted files by copying them from the main brach?

you should be able to do this:

git reset HEAD examples/research_projects/geodiff/geodiff_molecule_conformation.ipynb
git reset HEAD examples/research_projects/gligen/demo.ipynb

or

git reset -- examples/research_projects/geodiff/geodiff_molecule_conformation.ipynb
git reset --  examples/research_projects/gligen/demo.ipynb

you can also revert your last commit and unstage them.

but essentially it's the same as what you're proposing, just restore the files from main.

@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 25, 2025

@asomoza
Thanks for your patience.
OMG. I've made it finally.
I'll learn git as soon as possible.
BTW I hope nobody can see those commits.

Now only 4 files related to this PR has changed.

@CyberVy
Copy link
Contributor Author

CyberVy commented Feb 25, 2025

I've noticed that pipelines related to StableDiffusionControlnetPipeline have the same bug. This bug also causes the classes in diffusers.callback to not work properly. I think I will fix the bug about them in another PR. This PR only fixes SXDL pipeline related bug.

I will fix the other bugs I've mentioned before as soon as possible.

@asomoza
Copy link
Member

asomoza commented Feb 25, 2025

thanks a lot!

@asomoza asomoza merged commit 613e77f into huggingface:main Feb 25, 2025
12 checks passed
@CyberVy CyberVy deleted the sdxl_controlnet_inpaint_and_img2img_callback_tensor_inputs branch February 25, 2025 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants